added a conversion function from uri to iceorxy2 addressing scheme by catappreciationhours2 · Pull Request #71 · eclipse-uprotocol/up-transport-iceoryx2-rust

catappreciationhours2 · 2025-07-13T07:53:15Z

Helper methods:

compute_service_name to compute correct Iceoryx2 service names from UMessage
encode_uuri_segments and encode_hex_no_leading_zeros to format UUri segments

Unit tests covering service name computation for:

publish
notification
RPC request
RPC response

missing URIs (return proper UCode::INVALID_ARGUMENT)

This sets up the foundation for integrating uProtocol transport over Iceoryx2.

Closes #66

PLeVasseur · 2025-07-13T12:50:01Z

@catappreciationhours2 -- thanks for opening this!

Closes #66

Reading through this quickly, I think this should be:

Closes #14, #15

Since in this PR you've got implementation and testing of the UUri => ServiceName. I didn't spot implementation of handling of the payload. Let me know if I missed something ☺️

PLeVasseur

Hey @catappreciationhours2 -- thanks for putting up this PR. ☺️

I've left some suggestions. Could you take a look?

If possible I'd like us to have one go-round with you and the other students attempting to make changes / post comments here before we meet up on Monday. Seem doable?

PLeVasseur · 2025-07-13T14:12:44Z

+    }
+
+    fn compute_service_name(message: &UMessage) -> Result<String, UStatus> {
+        let join_segments = |segments: Vec<String>| segments.join("/");


Neat approach. Think this is probably fine.

Was wondering if you'd considered using a function defined local to this function vs using a closure like this. Wondered on if there are any trade-offs between the two. I don't know of any off-hand and it might be useful for your learning.

sophokles73

Please also make sure to format your code properly using

cargo fmt -all

or (better) configure VSCode to automatically format your code when saving a file.

sophokles73 · 2025-07-17T07:03:55Z

@catappreciationhours2 please make sure to use the email address in your commits that you have used to register your Eclipse Foundation account and that you have signed the Eclipse Contributor Agreement (ECA) for.

Otherwise, the eclipsefdn/eca check will always fail and we cannot merge your PR(s).

vidishac2004 · 2025-07-17T15:04:06Z

All the changes have been incorporated in the most latest commit by @catappreciationhours2 . Just a few questions about our implementation of compute service names:

Is using the type a good idea for the sink UURI's since publish messages don't necessary have one?
In the serviceName RESPONSE example- 403AB is truncated to 3AB, do we need to carry out truncation? Becuase from the examples provided to us in the spec, the ue_type is 403AB, but if i enter that in the test, it shows overflow, so do we have to alter our code for those cases?
Do we need to check for the fact that if publish sink is provided, it must be equal to the authority name?

PLeVasseur

Hi @catappreciationhours2 -- thanks for the adjustments!

I left a few more comments. In general I think making use of the provided helper functions on UUri as @sophokles73 pointed out can help with readability of the code.

PLeVasseur · 2025-07-18T01:34:30Z

Hey @vidishac2004, let's tackle these questions:

Is using the type a good idea for the sink UURI's since publish messages don't necessary have one?

I'm not sure I understand the question here. A Publish message doesn't have to have a sink UUri, it's true. We can check for presence or absence of the sink UUri to give us a hint on if it is not or is a Publish message.

If there's a sink UUri we can then check that the authority is the "host" authority.

In the serviceName RESPONSE example- 403AB is truncated to 3AB, do we need to carry out truncation? Becuase from the examples provided to us in the spec, the ue_type is 403AB, but if i enter that in the test, it shows overflow, so do we have to alter our code for those cases?

So taking these source and sink UUris:

Item	Result
Source URI	up://device1/CD/4/B
Sink URI	up://device1/403AB/3/0
iceoryx2 ServiceName	up/device1/CD/0/4/B/device1/3AB/4/3/0

I tried to explain how we end up with the ServiceName that we do:

I did this breakdown from ue_id to service_id and instance_id based on the uEntity definition given in the spec.

Do we need to check for the fact that if publish sink is provided, it must be equal to the authority name?

Yes, that seems reasonable. If a sink UUri is provided the authority must be equal to the "host" authority we can configure when making the Iceoryx2Transport.

PLeVasseur · 2025-07-18T14:22:59Z

Amending my earlier comment a bit:

If there's a sink UUri we can then check that the authority is the "host" authority.

Yes, that seems reasonable. If a sink UUri is provided the authority must be equal to the "host" authority we can configure when making the Iceoryx2Transport.

The situation here is a little more involved. I'll write something up about this. For now, I think we should check that if there is a sink UUri, that the sink UUri has only the authority_name field set.

I need to educate myself a little more on e.g. why the LocalUriProvider trait is no longer implemented in up-transport-zenoh-rust. I think I'm out of the loop.

PLeVasseur · 2025-07-18T14:23:06Z

Additionally, I'm a little unclear on the Publish section of the UAttributes spec portion where it says that the sink UUri

MAY be present in the message. If present, the UUri MUST only contain the authority_name field.

As far as I know in Protobuf the way we've defined UUri the other fields would still be present. Can you help sanity check me here @sophokles73?

…tests

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

…essage

Co-authored-by: Kai Hudalla <sophokles.kh@gmail.com>

…matching and case branches

…ge type

sophokles73 · 2025-07-21T06:44:53Z

If a sink UUri is provided the authority must be equal to the "host" authority we can configure when making the Iceoryx2Transport.

No, the reason for allowing an authority name in the sink of Publish messages was to support the streamer use case so that the streamer can (canonically) route messages to outbound transports, in particular to an off-vehicle message broker when using the MQTT5 transport for connecting the vehicle to a cloud backend.

IIRC the idea was, that the streamer would use information from its local uSubscription service to determine if the back end is interested in events produced locally in the vehicle and then add a sink URI containing the backend's authority name into such Publish messages before routing the message to the outbound transport. The MQTT5 transport would then use this information to create the topic name that the MQTT5 spec prescribes for off-vehicle communication.

PLeVasseur

Hey @catappreciationhours2 -- thanks for the revision. I've taken a look and offered up some suggestions. Can you take a peek?

PLeVasseur · 2025-07-21T21:34:51Z

+        assert_eq!(name, "up/device1/CD/0/4/B/device1/3AB/4/3/0");
+    }
+
+    // performing failing tests for service name computation


These look reasonable. Would encourage you to think more about all the ways compute_service_name() can fail and try to cover those various branches.

If you've already covered -- great! If not, would be good to add those too.

Leaving this open to see if you've given thought and covered all the possible branches.

…ogic

PLeVasseur

Hey @vidishac2004 -- nearing completion! I left a few comments and the build is failing.

PLeVasseur · 2025-07-25T02:35:11Z

+        assert_eq!(name, "up/device1/CD/0/4/B/device1/3AB/4/3/0");
+    }
+
+    // performing failing tests for service name computation


Leaving this open to see if you've given thought and covered all the possible branches.

PLeVasseur · 2025-07-25T02:38:47Z

+        let src_id = source.resource_id;
+        let sink_id = sink.map(|s| s.resource_id);
+
+        if src_id == 0 {


I think you could have kept the match, but refactored a bit to avoid the .contains().

This is a bit "wordy", but gets the job done.

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

vidishac2004 · 2025-07-25T12:06:09Z

Hey @vidishac2004 -- nearing completion! I left a few comments and the build is failing.

This has now been resolved.

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

PLeVasseur requested changes Jul 13, 2025

View reviewed changes

sophokles73 reviewed Jul 14, 2025

View reviewed changes

Comment thread .github/workflows/check.yml Outdated

Comment thread src/lib.rs Outdated

Comment thread src/lib.rs Outdated

sophokles73 reviewed Jul 17, 2025

View reviewed changes

Comment thread src/lib.rs Outdated

PLeVasseur requested changes Jul 18, 2025

View reviewed changes

Comment thread src/lib.rs Outdated

Comment thread src/lib.rs Outdated

Comment thread src/lib.rs Outdated

catappreciationhours2 and others added 5 commits July 21, 2025 01:10

added a conversion function from uri to iceorxy2 addressing scheme + …

ef9f7fb

…tests

Update src/lib.rs

f0d3f04

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

modified compute_service_name to accept source and sink instead of UM…

d99be0c

…essage

Update src/lib.rs

0f05535

Co-authored-by: Kai Hudalla <sophokles.kh@gmail.com>

modified to use helper function and use match for more clear pattern …

d71293a

…matching and case branches

catappreciationhours2 force-pushed the conversion branch from e5f5afa to d71293a Compare July 20, 2025 19:43

catappreciationhours2 added 2 commits July 21, 2025 02:11

modified compute service name to use custom helper to determine messa…

d29b055

…ge type

removed unwrap

bb007c9

PLeVasseur requested changes Jul 21, 2025

View reviewed changes

vidishac2004 added 4 commits July 24, 2025 03:55

trying to get rid of eol errors

8fa1894

trying to get rid of build errors due to Ustatus::invalid_argument

dc2bbac

replaced match with simple if/else statements and used the let else l…

a0eee80

…ogic

Normalize line endings to LF and add .gitattributes

7ce6f0c

PLeVasseur reviewed Jul 25, 2025

View reviewed changes

vidishac2004 and others added 3 commits July 25, 2025 17:33

resolved build error

b4be668

Update .gitignore

213d5ae

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

Update src/lib.rs

fc4905f

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

Update src/lib.rs

9677c8b

Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

sophokles73 merged commit 0bceeb2 into eclipse-uprotocol:main Jul 25, 2025
2 checks passed

Conversation

catappreciationhours2 commented Jul 13, 2025

Uh oh!

PLeVasseur commented Jul 13, 2025

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PLeVasseur Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sophokles73 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sophokles73 commented Jul 17, 2025

Uh oh!

vidishac2004 commented Jul 17, 2025

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PLeVasseur commented Jul 18, 2025

Uh oh!

PLeVasseur commented Jul 18, 2025

Uh oh!

PLeVasseur commented Jul 18, 2025

Uh oh!

sophokles73 commented Jul 21, 2025

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PLeVasseur Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

PLeVasseur Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

PLeVasseur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PLeVasseur Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

PLeVasseur Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

vidishac2004 commented Jul 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees